Skip to content

Allow hiding "Keep formatting" in "dont_override" config#9704

Closed
JE4GLE wants to merge 11 commits intoroundcube:masterfrom
Promatur:Patch-9703
Closed

Allow hiding "Keep formatting" in "dont_override" config#9704
JE4GLE wants to merge 11 commits intoroundcube:masterfrom
Promatur:Patch-9703

Conversation

@JE4GLE
Copy link
Contributor

@JE4GLE JE4GLE commented Nov 15, 2024 •
edited by github-actions bot
Loading

Fixes #9703
According to this blog entry, "Keep formatting" is for power users and therefore it would be nice to be removable.

@pabzm
Copy link
Member

pabzm commented Nov 18, 2024

Thank you for the PR! Please rebase onto the latest state of the "master" branch and remove the translation-related commits.

@JE4GLE
Copy link
Contributor Author

JE4GLE commented Nov 18, 2024 •
edited
Loading

@pabzm

Thank you for the PR! Please rebase onto the latest state of the "master" branch and remove the translation-related commits.

I rebased to the latest commit. I'm unsure on how to remove commits from the history. I already undid them and they are not part of the change. Do you want me to close this PR, create a new branch with a new PR?

@alecpl
Copy link
Member

alecpl commented Nov 19, 2024

This is incomplete. Well, maybe a rare case, but what if you set keep_formatting=true, and then add it to dont_override? A small change in rcmail_action_mail_send::run() is needed to support this case.

@JE4GLE
Copy link
Contributor Author

JE4GLE commented Nov 19, 2024

@alecpl

This is incomplete. Well, maybe a rare case, but what if you set keep_formatting=true, and then add it to dont_override? A small change in rcmail_action_mail_send::run() is needed to support this case.

Thank you, I changed it :)

alecpl
alecpl requested changes Nov 20, 2024
View reviewed changes
pabzm
pabzm approved these changes Nov 20, 2024
View reviewed changes
Copy link
Member

@pabzm pabzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@JE4GLE We need a line in the Changelog, would you maybe add one?

@pabzm
Copy link
Member

pabzm commented Nov 20, 2024

To be merged on 2024-12-04 unless someone comments.

cebe
cebe reviewed Nov 20, 2024
View reviewed changes
@pabzm pabzm self-requested a review November 20, 2024 11:21
pabzm
pabzm requested changes Nov 20, 2024
View reviewed changes
Copy link
Member

@pabzm pabzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to mention the actual config mechanism in the Changelog

@JE4GLE @pabzm
Update CHANGELOG.md
dc2c542 
Co-authored-by: Pablo Zmdl <[email protected]>
pabzm
pabzm approved these changes Nov 20, 2024
View reviewed changes
Copy link
Member

@pabzm pabzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much! 👍

@pabzm
Copy link
Member

pabzm commented Nov 20, 2024

/remind me to merge on 2024-12-04

@github-actions
Copy link

github-actions bot commented Nov 20, 2024

@pabzm set a reminder for 12/4/2024

@github-actions
Copy link

github-actions bot commented Dec 4, 2024

👋 @pabzm, merge

@github-actions github-actions bot removed the reminder label Dec 4, 2024
@Promatur Promatur closed this by deleting the head repository Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alecpl alecpl alecpl requested changes

@pabzm pabzm pabzm approved these changes

+1 more reviewer

@cebe cebe cebe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow hiding "Keep formatting" in "dont_override" config

5 participants

@JE4GLE @pabzm @alecpl @cebe @Promatur